Skip to content

Conversation

compiler-errors
Copy link
Member

This fixes the root-cause of the attached issues -- the root problem is that we're using the return type from a signature with late-bound instead of early-bound regions. The change on line 1087 (let Some(liberated_sig) = typeck_results.liberated_fn_sigs().get(fn_hir_id) else { return false; };) makes sure we're grabbing the right return type for this suggestion to check the dyn predicates with.

Fixes #91801
Fixes #91803

This fix also includes some drive-by changes, specifically:

  1. Don't suggest boxing when we have -> dyn Trait and are already returning Box<T> where T: Trait (before we always boxed the value).
  2. Suggestion applies even when the return type is a type alias (e.g. type Foo = dyn Trait). This does cause the suggestion to expand to the aliased type, but I think it's still beneficial.
  3. Split up the multipart suggestion because there's a 6-line max in the printed output...

I am open to splitting out the above changes, if we just want to fix the ICE first.

cc: @terrarier2111 and #92289

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 2, 2022
@rust-highfive
Copy link
Contributor

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2022
@rust-log-analyzer

This comment has been minimized.

@michaelwoerister
Copy link
Member

Thanks for the PR, @compiler-errors!

This looks like r? rust-lang/diagnostics to me.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 4, 2022

oooh nice

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 4, 2022

📌 Commit b899251 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 4, 2022
Fix late-bound ICE in `dyn` return type suggestion

This fixes the root-cause of the attached issues -- the root problem is that we're using the return type from a signature with late-bound instead of early-bound regions. The change on line 1087 (`let Some(liberated_sig) = typeck_results.liberated_fn_sigs().get(fn_hir_id) else { return false; };`) makes sure we're grabbing the _right_ return type for this suggestion to check the `dyn` predicates with.

Fixes rust-lang#91801
Fixes rust-lang#91803

This fix also includes some drive-by changes, specifically:

1. Don't suggest boxing when we have `-> dyn Trait` and are already returning `Box<T>` where `T: Trait` (before we always boxed the value).
2. Suggestion applies even when the return type is a type alias (e.g. `type Foo = dyn Trait`). This does cause the suggestion to expand to the aliased type, but I think it's still beneficial.
3. Split up the multipart suggestion because there's a 6-line max in the printed output...

I am open to splitting out the above changes, if we just want to fix the ICE first.

cc: `@terrarier2111` and rust-lang#92289
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 4, 2022
Fix late-bound ICE in `dyn` return type suggestion

This fixes the root-cause of the attached issues -- the root problem is that we're using the return type from a signature with late-bound instead of early-bound regions. The change on line 1087 (`let Some(liberated_sig) = typeck_results.liberated_fn_sigs().get(fn_hir_id) else { return false; };`) makes sure we're grabbing the _right_ return type for this suggestion to check the `dyn` predicates with.

Fixes rust-lang#91801
Fixes rust-lang#91803

This fix also includes some drive-by changes, specifically:

1. Don't suggest boxing when we have `-> dyn Trait` and are already returning `Box<T>` where `T: Trait` (before we always boxed the value).
2. Suggestion applies even when the return type is a type alias (e.g. `type Foo = dyn Trait`). This does cause the suggestion to expand to the aliased type, but I think it's still beneficial.
3. Split up the multipart suggestion because there's a 6-line max in the printed output...

I am open to splitting out the above changes, if we just want to fix the ICE first.

cc: ``@terrarier2111`` and rust-lang#92289
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#91873 (Mention implementers of unsatisfied trait)
 - rust-lang#95588 (explicitly distinguish pointer::addr and pointer::expose_addr)
 - rust-lang#95603 (Fix late-bound ICE in `dyn` return type suggestion)
 - rust-lang#95620 (interpret: remove MemoryExtra in favor of giving access to the Machine)
 - rust-lang#95630 (Update `NonNull` pointer provenance methods' documentation)
 - rust-lang#95631 (Refactor: remove unnecessary nested blocks)
 - rust-lang#95642 (`CandidateSource::XCandidate` -> `CandidateSource::X`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 92e53f5 into rust-lang:master Apr 5, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 5, 2022
@compiler-errors compiler-errors deleted the dyn-return branch April 7, 2022 04:29
@pnkfelix pnkfelix mentioned this pull request Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
8 participants